fix(agglayer): fix incorrect stack comments by partylikeits1983 · Pull Request #2681 · 0xMiden/protocol

partylikeits1983 · 2026-03-24T10:46:14Z

This PR fixes incorrect stack comments found in crates/miden-agglayer/asm/.

Summary:

1. `bridge_in.masm` — Missing `DEFAULT_TAG` after push

Location: create_mint_note_with_attachment

After push.DEFAULT_TAG, the comment omitted the newly pushed value:

  push.DEFAULT_TAG
- # => [note_type, MINT_RECIPIENT, faucet_id_suffix, faucet_id_prefix]
+ # => [tag, note_type, MINT_RECIPIENT, faucet_id_suffix, faucet_id_prefix]

2. `bridge_in.masm` — prefix/suffix order swapped in three procedures

Location: build_mint_output_note, create_mint_note_with_attachment

Comments throughout the MINT note creation flow had faucet_id_prefix and faucet_id_suffix in the wrong order. The actual stack has suffix on top (matching key_to_faucet_id's output), but comments said prefix was on top. This was as a result of the VM stack ordering update, these were just comments that didn't get updated.

  exec.write_mint_note_storage
- # => [faucet_id_prefix, faucet_id_suffix]
+ # => [faucet_id_suffix, faucet_id_prefix]

  exec.build_mint_recipient
- # => [MINT_RECIPIENT, faucet_id_prefix, faucet_id_suffix]
+ # => [MINT_RECIPIENT, faucet_id_suffix, faucet_id_prefix]

- #! Inputs:  [MINT_RECIPIENT, faucet_id_prefix, faucet_id_suffix]
+ #! Inputs:  [MINT_RECIPIENT, faucet_id_suffix, faucet_id_prefix]

- # network_account_target::new expects [prefix, suffix, exec_hint]
+ # network_account_target::new expects [suffix, prefix, exec_hint]

All downstream comments within create_mint_note_with_attachment were updated to match.

3. `leaf_utils.masm` — Variable name typo (`curr_lsb` → `curr_msb`)

Location: pack_leaf_data

The value at that stack position is the result of u32shr.24, which extracts the upper 8 bits (MSB) of the current u32 element. The existing comment on line 97 already describes this as "extract MSB (upper 8 bits)", and line 109 references it as curr_msb. The intermediate comments on lines 103 and 107 were inconsistent, using curr_lsb instead:

- # => [next_src_addr, curr_lsb, curr_elem, counter]
+ # => [next_src_addr, curr_msb, curr_elem, counter]

- # => [next_elem, curr_lsb, curr_elem, counter]
+ # => [next_elem, curr_msb, curr_elem, counter]

4. `bridge_out.masm` — Padding cleanup count

Location: convert_asset

The FPI call to asset_to_origin_asset returns [U256(8), addr(5), origin_network, pad(2)] — exactly 2 padding elements. The cleanup was corrected from repeat.3 to repeat.2 and the comment updated accordingly.

crates/miden-agglayer/asm/agglayer/bridge/bridge_out.masm

Fumuran

Looks good, thank you! I left two questions inline

Fumuran · 2026-03-29T16:03:27Z

crates/miden-agglayer/asm/agglayer/bridge/bridge_out.masm

    repeat.2
        padw padw swapdw
    end
    # => [ASSET_KEY, ASSET_VALUE, pad(16)]


Judging by the code below, we will use almost all of the 16 pads here (only 15 of them, to be precise). Not sure why it works now, but probably we should add another padw padw swapdw here

I am not sure about this, because we add 16 pad elements here before doing the FPI call.

Ah, my bad, somehow I missed that we already have repeat.2 here. Yep, in that case all looks good!

crates/miden-agglayer/asm/agglayer/bridge/bridge_out.masm

Fumuran

Looks great, thank you!

Fumuran · 2026-03-30T10:53:51Z

crates/miden-agglayer/asm/agglayer/bridge/bridge_out.masm

    repeat.2
        padw padw swapdw
    end
    # => [ASSET_KEY, ASSET_VALUE, pad(16)]


Ah, my bad, somehow I missed that we already have repeat.2 here. Yep, in that case all looks good!

* refactor(agglayer): fix minor formatting issues (#2675) * refactor: fix minor formatting issues * fix: cleanup unecessary stack comments * fix: add newline * refactor: rename U256[N] to U256_LO/U256_HI in Agglayer MASM comments (#2676) * refactor: fix minor formatting issues * fix: cleanup unecessary stack comments * fix: add newline * Initial plan * refactor: rename U256[0]/U256[1] to U256_LO/U256_HI in Agglayer MASM comments Co-authored-by: mmagician <8402446+mmagician@users.noreply.github.com> Agent-Logs-Url: https://github.com/0xMiden/protocol/sessions/604803e2-68c0-4ffe-acdd-4926707426f5 * refactor: remove (4) size suffix from AMOUNT_U256_LO/HI in bridge_out.masm comments Co-authored-by: mmagician <8402446+mmagician@users.noreply.github.com> Agent-Logs-Url: https://github.com/0xMiden/protocol/sessions/ab585523-16e0-40fd-b99b-e64a472ef9d0 --------- Co-authored-by: riemann <aleqvids@gmail.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mmagician <8402446+mmagician@users.noreply.github.com> * fix: rm unused procedure (#2680) * fix(agglayer): update doc & stack comments (#2678) * fix: update stack comment in claim_batch_pipe_double_words * fix: update doc comments & formatting * fix(agglayer): remove pub proc definition from internal procs (#2679) * fix: remove pub proc definition from internal procs * fix: add stack comment & newline * AggLayer audit preparation fixes (#2686) * refactor: B2AGG: optimize stack management, update format, remove unused imports * refactor: CLAIM note: remove unused imports, update code format * refactor: CONFIG_AGG_BRIDGE: update code and formatting, remove unused imports * refactor: UPDATE_GER: remove unused imports, update format * chore: update comments format for agglayer account components --------- Co-authored-by: Marti <marti@miden.team> * Agglayer modules format unification (#2694) * refactor: faucet account: remove unused imports, update comments * refactor: update format of the bridge_config * refactor: update format in bridge_in * refactor: fix addr order in config note inline docs * refactor: update format of the bridge_out * refactor: update format of the leaf_utils, move EthereumAddressFormat to eth_address * chore: small format fixes * fix(agglayer): fix incorrect stack comments (#2681) * fix: fix incorrect stack comments * fix: rm plan md file * fix: undo incorrect padding drop * fix: update comment * chore: update AggLayer spec (#2710) * docs(agglayer): inline permissions into note type sections Move the standalone "Current permissions" table from section 1 into per-note-type "Permissions" sub-sections within section 3, making authorization details easier to find when reading about each note type. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs(agglayer): add Protocol Description section with audit TODOs Add a new Section 2 describing the high-level protocol flows (bridge-out, bridge-in, GER lifecycle, faucet registration, administration) with TODO markers linking to audit issue files for identified gaps relative to the Solidity v2 contracts. Renumber subsequent sections accordingly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: add flow diagrams * chore: clarify who monitors L1 --------- Co-authored-by: Claude (Opus) <noreply@anthropic.com> --------- Co-authored-by: Alexander John Lee <77119221+partylikeits1983@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: riemann <aleqvids@gmail.com> Co-authored-by: Andrey Khmuro <andrey@polygon.technology> Co-authored-by: Claude (Opus) <noreply@anthropic.com> Co-authored-by: Bobbin Threadbare <43513081+bobbinth@users.noreply.github.com>

fix: fix incorrect stack comments

2800d9b

partylikeits1983 requested review from Fumuran and mmagician March 24, 2026 10:46

partylikeits1983 added no changelog This PR does not require an entry in the `CHANGELOG.md` file agglayer PRs or issues related to AggLayer bridging integration pr-from-maintainers PRs that come from internal contributors or integration partners. They should be given priority labels Mar 24, 2026

partylikeits1983 changed the title ~~fix: fix incorrect stack comments~~ fix(agglayer): fix incorrect stack comments Mar 24, 2026

partylikeits1983 marked this pull request as ready for review March 24, 2026 10:47

mmagician reviewed Mar 27, 2026

View reviewed changes

crates/miden-agglayer/asm/agglayer/bridge/bridge_out.masm Outdated Show resolved Hide resolved

chore: merge latest agglayer

8c1faa3

mmagician mentioned this pull request Mar 29, 2026

chore: AggLayer tracking branch #2695

Merged

Fumuran reviewed Mar 29, 2026

View reviewed changes

partylikeits1983 and others added 4 commits March 29, 2026 18:12

fix: rm plan md file

fc9b7e0

fix: undo incorrect padding drop

029b364

Merge branch 'agglayer' into ajl-fix-stack-comments

ce4f66f

fix: update comment

c1a7898

Fumuran approved these changes Mar 30, 2026

View reviewed changes

mmagician approved these changes Mar 30, 2026

View reviewed changes

mmagician added this pull request to the merge queue Mar 30, 2026

Merged via the queue into agglayer with commit 707d487 Mar 30, 2026
17 checks passed

mmagician deleted the ajl-fix-stack-comments branch March 30, 2026 12:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(agglayer): fix incorrect stack comments#2681

fix(agglayer): fix incorrect stack comments#2681
mmagician merged 6 commits intoagglayerfrom
ajl-fix-stack-comments

partylikeits1983 commented Mar 24, 2026

Uh oh!

Uh oh!

Fumuran left a comment

Uh oh!

Fumuran Mar 29, 2026

Uh oh!

partylikeits1983 Mar 30, 2026

Uh oh!

Fumuran Mar 30, 2026

Uh oh!

Uh oh!

Fumuran left a comment

Uh oh!

Fumuran Mar 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

partylikeits1983 commented Mar 24, 2026

Summary:

1. bridge_in.masm — Missing DEFAULT_TAG after push

2. bridge_in.masm — prefix/suffix order swapped in three procedures

3. leaf_utils.masm — Variable name typo (curr_lsb → curr_msb)

4. bridge_out.masm — Padding cleanup count

Uh oh!

Uh oh!

Fumuran left a comment

Choose a reason for hiding this comment

Uh oh!

Fumuran Mar 29, 2026

Choose a reason for hiding this comment

Uh oh!

partylikeits1983 Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Fumuran Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Fumuran left a comment

Choose a reason for hiding this comment

Uh oh!

Fumuran Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

1. `bridge_in.masm` — Missing `DEFAULT_TAG` after push

2. `bridge_in.masm` — prefix/suffix order swapped in three procedures

3. `leaf_utils.masm` — Variable name typo (`curr_lsb` → `curr_msb`)

4. `bridge_out.masm` — Padding cleanup count